-
Notifications
You must be signed in to change notification settings - Fork 85
export: initial work to support MemSQL/SingleStore #311
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
The type and version detection in Dumpling is based on
|
ac633d4
to
b529dfb
Compare
Issue: pingcap#309 - `WITH CONSISTENT SNAPSHOT` is not supported by MemSQL (same for Vitess). - `SHOW CREATE DATABASE` is not supported by MemSQL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with MemSQL, could you briefly post your test result in this PR?
@@ -12,6 +12,8 @@ import ( | |||
"strconv" | |||
"strings" | |||
|
|||
"github.com/go-sql-driver/mysql" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe group the 3-rd party packages
I used a cluster-in-a-box test setup which is basically MemSQL in a container.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the result of select version()
for MemSQL
? Can we use special SQL for different databases at first?
@@ -642,7 +651,14 @@ func createConnWithConsistency(ctx context.Context, db *sql.DB) (*sql.Conn, erro | |||
query = "START TRANSACTION /*!40108 WITH CONSISTENT SNAPSHOT */" | |||
_, err = conn.ExecContext(ctx, query) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check the error type here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could try to detect the error type and check for the error number here. However I think retrying without the optional part should give the same result. I don't think the extra complexity is needed.
5.5.58 127.0.0.1:3306 test SQL START TRANSACTION /*!40108 WITH CONSISTENT SNAPSHOT */;
ERROR: 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WITH CONSISTENT SNAPSHOT */' at line 1
5.5.58 127.0.0.1:3306 test SQL SELECT @@version, @@memsql_version;
+-----------+------------------+
| @@version | @@memsql_version |
+-----------+------------------+
| 5.5.58 | 7.3.13 |
+-----------+------------------+
1 row in set (0.0357 sec)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see
I already posted this:
Both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 4c05065
|
In response to a cherrypick label: new pull request created: #327. |
In response to a cherrypick label: new pull request created: #328. |
In response to a cherrypick label: new pull request created: #329. |
What problem does this PR solve?
Issue: #309
WITH CONSISTENT SNAPSHOT
is not supported by MemSQL (same for Vitess).SHOW CREATE DATABASE
is not supported by MemSQL.What is changed and how it works?
Check List
Tests
Release note
START TRANSACTION ... WITH CONSISTENT SNAPSHOT
SHOW CREATE TABLE